Skip to content

Assemblyscan 1.0.0#9506

Merged
eit-maxlcummins merged 20 commits intonf-core:masterfrom
eit-maxlcummins:assemblyscan_1.0.0
Jan 15, 2026
Merged

Assemblyscan 1.0.0#9506
eit-maxlcummins merged 20 commits intonf-core:masterfrom
eit-maxlcummins:assemblyscan_1.0.0

Conversation

@eit-maxlcummins
Copy link
Contributor

@eit-maxlcummins eit-maxlcummins commented Dec 4, 2025

Description

This updates the assemblyscan module to v1.0.0 from v0.4.0, enabling tsv or json output.

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • [-] If you've added a new tool - have you followed the module conventions in the contribution docs
  • [-] If necessary, include test data in your PR.
  • Remove all TODO statements.
  • Emit the versions.yml file.
  • Follow the naming conventions.
  • Follow the parameters requirements.
  • Follow the input/output options guidelines.
  • [-] Add a resource label
  • Use BioConda and BioContainers if possible to fulfil software requirements.
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • For modules:
      • nf-core modules test <MODULE> --profile docker
      • [-] nf-core modules test <MODULE> --profile singularity
      • nf-core modules test <MODULE> --profile conda

@prototaxites
Copy link
Contributor

prototaxites commented Dec 15, 2025

@nf-core-bot fix linting

@prototaxites
Copy link
Contributor

prototaxites commented Dec 15, 2025

This looks really good, but can we add a second test to validate JSON output?

SPPearce
SPPearce reviewed Jan 14, 2026
View reviewed changes
Copy link
Contributor

@SPPearce SPPearce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check, was this previously producing a tsv output but calling it a .json file? ☹️

It'd be nice if you could swap to using a topic channel to capture the version too: https://nf-co.re/blog/2025/version_topics

@charles-plessy
Copy link
Contributor

charles-plessy commented Jan 15, 2026 •
edited
Loading

Hi all, I am jumping in as I want to update this module in https://github.com/nf-core/pairgenomealign. assenbly-scan used to default to JSON but now changed to TSV, which is why the format changed in the test snapshot in this PR. I have prepared 3 commits that:

  • add tests for JSON output,
  • switch to version topic channel, and
  • add a stub.
    I thought I could push them to the PR but failed and created a new branch instead (master...pr-9506). Please let me know your thoughts.

@eit-maxlcummins
Copy link
Contributor Author

eit-maxlcummins commented Jan 15, 2026

@charles-plessy - thanks for your contributions. I have cherry picked your commits and added them to this PR and added you to the authorship in the meta :)

@eit-maxlcummins
Copy link
Contributor Author

eit-maxlcummins commented Jan 15, 2026

Just to check, was this previously producing a tsv output but calling it a .json file? ☹️

It'd be nice if you could swap to using a topic channel to capture the version too: https://nf-co.re/blog/2025/version_topics

Done :)

SPPearce
SPPearce reviewed Jan 15, 2026
View reviewed changes
Copy link
Contributor

@SPPearce SPPearce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we just want to capture the output in one channel, rather than two separate ones? That would be within guidelines: https://nf-co.re/docs/guidelines/components/modules#one-output-channel-per-output-file-type

dwells-eit
dwells-eit approved these changes Jan 15, 2026
View reviewed changes
Copy link
Contributor

@dwells-eit dwells-eit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 😎

@charles-plessy
Copy link
Contributor

charles-plessy commented Jan 15, 2026

@nf-core-bot fix linting

Regarding the lint failures, I have not managed to get the tests pass with topic channels when the evaluated shell command has a $ sign in. But in the case of assemblyscan, we do not need one: eval("assembly-scan --version | sed 's/assembly-scan //'") works well enough, and then the meta.yml files can contain - "assembly-scan --version | sed 's/assembly-scan //'": without triggering errors.

@eit-maxlcummins
Copy link
Contributor Author

eit-maxlcummins commented Jan 15, 2026 •
edited
Loading

@nf-core-bot fix linting

Regarding the lint failures, I have not managed to get the tests pass with topic channels when the evaluated shell command has a $ sign in. But in the case of assemblyscan, we do not need one: eval("assembly-scan --version | sed 's/assembly-scan //'") works well enough, and then the meta.yml files can contain - "assembly-scan --version | sed 's/assembly-scan //'": without triggering errors.

@charles-plessy Thanks! Was playing around with that exact issue right now. Passes locally but the GitHub runner runs a dev version of nf-core tools.

Will trim the regex to remove $ if it fails now. Thanks for sharing :)

@eit-maxlcummins eit-maxlcummins added this pull request to the merge queue Jan 15, 2026
Merged via the queue into nf-core:master with commit 947e154 Jan 15, 2026
29 checks passed
@eit-maxlcummins eit-maxlcummins deleted the assemblyscan_1.0.0 branch January 15, 2026 15:10
cavenel pushed a commit to cavenel/modules that referenced this pull request Feb 5, 2026
@eit-maxlcummins @nf-core-bot
@prototaxites @charles-plessy @SPPearce @cavenel
Assemblyscan 1.0.0 (nf-core#9506)
cc4b02e 
* rebase - git pull --rebase upstream master - merge conflicts

* add missing test for main.nf.test

* simplify string replacement for trinity version eval to remove use of colons - avoids potential yml clash

* update meta.yml to capture new version topics change

* update assemblyscan to v1.0.0, allowing json or tsv output

* [automated] Fix linting with Prettier

* update versions capturing to new approach and update snapshot

* add charles-plessy to authors

* combine outputs into single channel

* update meta.yml for dynamic report output and versions topic channel

* more fixes to meta.yml for versions topic

* more fixes to meta.yml for versions topic

* manually add json test from @charles-plessy - some prior git cherry pick issue from my end

* lint module

* remove unnecessary optional flag post output channel merge

* final meta.yml changes 🙏

* [automated] Fix linting with Prettier

---------

Co-authored-by: nf-core-bot <core@nf-co.re>
Co-authored-by: Jim Downie <19718667+prototaxites@users.noreply.github.com>
Co-authored-by: Charles Plessy <charles.plessy@oist.jp>
Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SPPearce SPPearce SPPearce approved these changes

@dwells-eit dwells-eit dwells-eit approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@eit-maxlcummins @prototaxites @charles-plessy @SPPearce @dwells-eit @nf-core-bot